Add Joins #57
Add Joins #57
Conversation
|
The more I think about the functionality of this the more I feel that it should have input similar to your filter method. Thoughts? |
paul-griffith
left a comment
paul-griffith
left a comment
There was a problem hiding this comment.
It might be worth a larger refactor approaching this from a slightly different angle to ease testing and implementation.
What if the different join functions were regular Kotlin functions that accept the two datasets and the column args.
Then the primary unit testing effort can be focused on the plain Kotlin methods (with no need for extra Python scaffolding), and can use DSBuilder to construct test datasets more easily.
Once the actual method implementations are proven well, it's easy to write scripting-focused methods that only have to care about parsing the args and passing them to the inner functions - and since there's a lot of shared boilerplate, that can be tested as one piece as well.
So it's 4 distinct tests for the join functions, plus 1 test of Python argument parsing, instead of 4 tests with lots of overlap for argument handling. Plus, well structured, the argument handling could probably be repeated in each script function implementation, which avoids extra work if requirements for the function(s) change.
| val column = parsedArgs.requirePyObject("columnIndex").toJava<Int>() | ||
| val column2 = parsedArgs.requirePyObject("columnIndex2").toJava<Int>() |
There was a problem hiding this comment.
Should be able to just do parsedArgs.requireInteger("columnIndex") to get a primitive int back; saves some work
| val listToAppend = arrayOfNulls<Any?>(combinedColumnName.size) | ||
| var row2: Int? = null | ||
|
|
||
| dataset2.rowIndices.forEachIndexed { rowIndex, _ -> |
There was a problem hiding this comment.
Maybe try firstNonNullOfOrNull {} here to express what you're trying to do more succintly
| names = ["dataset", "columnsToSplit"], | ||
| types = [Dataset::class, Array<Array<Int>>::class], | ||
| ) | ||
| fun splitter(args: Array<PyObject>, keywords: Array<String>): Array<Dataset?> { |
There was a problem hiding this comment.
Returning as an array is a little weird - a plain list would be more typical for Python/Java/Ignition
(cherry picked from commit 0cb0a58)
(cherry picked from commit 399777f)
(cherry picked from commit b291f05)
(cherry picked from commit 83dc405)
(cherry picked from commit 7853852)
# Conflicts: # common/build.gradle.kts # common/src/test/kotlin/org/imdc/extensions/common/DatasetExtensionsTests.kt
(cherry picked from commit 399777f)
(cherry picked from commit 7853852)
|
I plan to revamp this. Currently, my thought is, it would be nice to have something like This could be a powerful tool. My current hang-up is passing two datasets into |
|
I like that proposed syntax. You might like Phil Turmel's Simulation Aids; particularly the crazy stuff he's doing with expressions: https://forum.inductiveautomation.com/t/automation-professionals-simulation-aids-v2/74934
I don't think the Kotlin function should expect the Python function to accept kwargs at all. A join is effectively a function |
|
Thanks for pointing me back to that page. That was where I originally started my search. It appears 1 hour, and he added left join functionality 😆 https://forum.inductiveautomation.com/t/automation-professionals-simulation-aids-v2/74934/42 I'll give it a shot tomorrow. If it works, I'll leave it up to you to close this. I enjoy the learning experience of this and like how I have full customization and the fact that it currently works. However, I probably won't spend much time on it if this works, as I have a full platter and would only be able to work on it later. |
|
Okay, so this should be a working left join. It was a lot simpler than I thought. I still need to clean it up and put the join type as an option, but then it should be good to go after that (I believe). |
|
It should be good to go. I ended up going with column indexs instead of column names. I can change it if you want. I just though this was more elegant. Example of Left Join: |
|
I plan to make a change to this. I am not sure how I overlooked this, but for some reason, I had a |
|
A work-in-progress refactor of what I was alluding to earlier with breaking things out to make unit testing a bit easier: |
2 participants
Adds left, right, inner, and outer joins. It also has some tiny bug fixes.